[dashboard] feat: add auto_update_always toggle to client settings#580
[dashboard] feat: add auto_update_always toggle to client settings#580
Conversation
Add "Always Update" toggle to the Clients settings tab that controls whether updates are installed automatically in the background or require user interaction from the UI. Includes a warning icon and caution callout when enabled to highlight the risk of disrupting active connections.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new boolean setting Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Client
participant Server
participant Database
User->>Client: Toggle "Always Update"
Client->>Client: set autoUpdateAlways, mark hasChanges
User->>Client: Click "Save"
Client->>Server: PUT /accounts/:id { settings: {..., auto_update_always } }
Server->>Database: persist Account.settings.auto_update_always
Database-->>Server: ack
Server-->>Client: 200 OK (updated account)
Client->>Client: update UI state, clear hasChanges, trigger updateRef
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/interfaces/Account.ts (1)
29-29: Consider makingauto_update_alwaysoptional for backwards compatibility.Other recently added settings like
peer_expose_enabled,network_range, andlocal_auth_disableduse the optional modifier (?). If existing accounts don't have this field persisted yet, the API may returnundefined.The consuming code already handles this defensively with
account.settings?.auto_update_always ?? false, but the type should reflect the actual API contract.Suggested change
- auto_update_always: boolean; + auto_update_always?: boolean;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/interfaces/Account.ts` at line 29, The Account interface's auto_update_always field is declared as required but the API can return undefined; make auto_update_always optional to match other recent settings and the consuming code's defensive checks. Update the Account interface by changing the auto_update_always property to optional (like peer_expose_enabled, network_range, local_auth_disabled) so its type is boolean | undefined and ensure any references to Account.settings?.auto_update_always continue to use the existing nullish coalescing handling.src/modules/settings/ClientSettingsTab.tsx (1)
277-307: Simplify state reset logic when disabling auto-updates for better UX consistency.When
autoUpdateMethodchanges to"disabled", theautoUpdateAlwaysstate is not reset. This means the save payload includesauto_update_always: trueeven when updates are disabled, and re-enabling updates restores "Always Update" without explicit user action.To align with the existing pattern used for
autoUpdateCustomVersion, consider resettingautoUpdateAlwayswhen updates are disabled:Suggested refactor
const handleUpdateMethodChange = (value: string) => { setAutoUpdateMethod(value); if (value === "disabled" || value === "latest") { setAutoUpdateCustomVersion(""); } + if (value === "disabled") { + setAutoUpdateAlways(false); + } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/settings/ClientSettingsTab.tsx` around lines 277 - 307, In ClientSettingsTab, ensure autoUpdateAlways is cleared when autoUpdateMethod becomes "disabled": add a useEffect inside the ClientSettingsTab component that watches autoUpdateMethod and, when it === "disabled", calls setAutoUpdateAlways(false) (mirroring the existing reset for autoUpdateCustomVersion) so the save payload no longer contains auto_update_always:true while updates are disabled and toggling the method restores the correct initial state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/interfaces/Account.ts`:
- Line 29: The Account interface's auto_update_always field is declared as
required but the API can return undefined; make auto_update_always optional to
match other recent settings and the consuming code's defensive checks. Update
the Account interface by changing the auto_update_always property to optional
(like peer_expose_enabled, network_range, local_auth_disabled) so its type is
boolean | undefined and ensure any references to
Account.settings?.auto_update_always continue to use the existing nullish
coalescing handling.
In `@src/modules/settings/ClientSettingsTab.tsx`:
- Around line 277-307: In ClientSettingsTab, ensure autoUpdateAlways is cleared
when autoUpdateMethod becomes "disabled": add a useEffect inside the
ClientSettingsTab component that watches autoUpdateMethod and, when it ===
"disabled", calls setAutoUpdateAlways(false) (mirroring the existing reset for
autoUpdateCustomVersion) so the save payload no longer contains
auto_update_always:true while updates are disabled and toggling the method
restores the correct initial state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8b11ac19-71af-468f-891e-0e828bd270ad
📒 Files selected for processing (2)
src/interfaces/Account.tssrc/modules/settings/ClientSettingsTab.tsx
heisbrot
left a comment
heisbrot
left a comment
There was a problem hiding this comment.
We should update the description for automatic updates. Currently, it is not clear that it requires manual user interaction.
The Always Update toggle is not very clear. Maybe "Enable Silent Updates" / "Enable Unattended Updates" / "Force Automatic Updates" or something like that?
The callout inside the Always Update toggle looks strange. We can remove "This may interrupt..." sentence from the toggle switch and move the callout below the switch (show the callout only when always update is enabled, as it currently is the case)
Clarify that automatic updates require user interaction by updating the description. Rename "Always Update" to "Force Automatic Updates" for clarity. Move warning callout below the toggle switch instead of inside it.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/modules/settings/ClientSettingsTab.tsx`:
- Around line 89-91: The autoUpdateAlways state (autoUpdateAlways /
setAutoUpdateAlways) is never cleared when the update method is set to
"disabled", causing auto_update_always to be restored incorrectly; update the
component so that whenever account.settings?.auto_update_version (or the local
update method state) becomes "disabled" you call setAutoUpdateAlways(false) to
normalize the UI, and also ensure the save payload sent from the save handler
clears/persists auto_update_always as false when auto_update_version ===
"disabled" so the backend cannot store a true value alongside a disabled update
method.
- Around line 247-249: The UI text in ClientSettingsTab contains contradictory
copy about automatic updates; update the copy to match the selected "force"
toggle state: locate the descriptive strings rendered in the ClientSettingsTab
component for automatic updates (the paragraphs currently explaining "prompted
to install" and "install automatically") and make them conditional on the force
toggle state variable (e.g., when force is true show the automatic-installation
wording, when false show the user-prompt wording). Ensure both places that
display the automatic-update description use the same conditional text logic so
they never contradict each other.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 85e8f9d6-b86a-4760-b920-8c256de6a5d7
📒 Files selected for processing (1)
src/modules/settings/ClientSettingsTab.tsx
| const [autoUpdateAlways, setAutoUpdateAlways] = useState( | ||
| account.settings?.auto_update_always ?? false, | ||
| ); |
There was a problem hiding this comment.
Normalize autoUpdateAlways when auto updates are disabled.
This state is restored from Line 90 and always persisted again on Line 165, but nothing clears it when the update method becomes "disabled". A user can save auto_update_version: "disabled" together with auto_update_always: true, and after reload the force toggle comes back on but disabled.
💡 Suggested fix
const [autoUpdateAlways, setAutoUpdateAlways] = useState(
- account.settings?.auto_update_always ?? false,
+ account.settings?.auto_update_version !== "disabled" &&
+ (account.settings?.auto_update_always ?? false),
); const handleUpdateMethodChange = (value: string) => {
setAutoUpdateMethod(value);
if (value === "disabled" || value === "latest") {
setAutoUpdateCustomVersion("");
}
+ if (value === "disabled") {
+ setAutoUpdateAlways(false);
+ }
}; ...account.settings,
auto_update_version: autoUpdateCustomVersion || autoUpdateMethod,
- auto_update_always: autoUpdateAlways,
+ auto_update_always:
+ autoUpdateMethod === "disabled" ? false : autoUpdateAlways,
peer_expose_enabled: peerExposeEnabled,Also applies to: 164-166
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/settings/ClientSettingsTab.tsx` around lines 89 - 91, The
autoUpdateAlways state (autoUpdateAlways / setAutoUpdateAlways) is never cleared
when the update method is set to "disabled", causing auto_update_always to be
restored incorrectly; update the component so that whenever
account.settings?.auto_update_version (or the local update method state) becomes
"disabled" you call setAutoUpdateAlways(false) to normalize the UI, and also
ensure the save payload sent from the save handler clears/persists
auto_update_always as false when auto_update_version === "disabled" so the
backend cannot store a true value alongside a disabled update method.
| Configure how NetBird clients receive update notifications. | ||
| When enabled, users will be prompted to install the selected | ||
| version. Automatic Updates require at least NetBird{" "} |
There was a problem hiding this comment.
The new automatic-update copy contradicts itself.
Lines 247-249 say enabled updates prompt the user to install, while Lines 287-289 say they install automatically without user interaction. When the force toggle is on, both statements are shown together.
✏️ Possible wording
- Configure how NetBird clients receive update notifications.
- When enabled, users will be prompted to install the selected
- version. Automatic Updates require at least NetBird{" "}
+ Configure how NetBird clients receive updates. By default, users
+ are prompted to install the selected version, or you can enable
+ Force Automatic Updates below to install it in the background.
+ Automatic Updates require at least NetBird{" "}Also applies to: 287-289
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/settings/ClientSettingsTab.tsx` around lines 247 - 249, The UI
text in ClientSettingsTab contains contradictory copy about automatic updates;
update the copy to match the selected "force" toggle state: locate the
descriptive strings rendered in the ClientSettingsTab component for automatic
updates (the paragraphs currently explaining "prompted to install" and "install
automatically") and make them conditional on the force toggle state variable
(e.g., when force is true show the automatic-installation wording, when false
show the user-prompt wording). Ensure both places that display the
automatic-update description use the same conditional text logic so they never
contradict each other.
Co-authored-by: Eduard Gert <kontakt@eduardgert.de>
2 participants
Add "Always Update" toggle to the Clients settings tab that controls whether updates are installed automatically in the background or require user interaction from the UI. Includes a warning icon and caution callout when enabled to highlight the risk of disrupting active connections.
Summary by CodeRabbit